-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Avoid the use/creation of real databases in the UTs. #570
Avoid the use/creation of real databases in the UTs. #570
Conversation
e734d9b
to
fe80020
Compare
db233e0
to
aa8aa7f
Compare
aa8aa7f
to
f73d423
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if we are doing the correct thing here by testing AgentInfo and AgentInfoRegistration with a mocked persistence class. The specific of the persistence implementation should be a detail visible only to AgentInfoPersistence, who uses it. In some of these tests we are actually testing the certain Persistence functions are being called, which I believe should be beyond the concerns of AgentInfo and AgentInfoRegistration. Maybe it would be more correct to test AgentInfo with a mocked AgentInfoPersistence, and to test AgentInfoRegistration with a mocked AgentInfo? Perhaps you already evaluated this, let me know what you think.
…Persistence object to AgentInfoPersistance
…t, SetName, SetKey and SetUUID functions
…saction/RollBack fails in the SetGroups method
… ones for using the Persistence mock
…to inject an AgentInfo object. The AgentRegistration UTs has also been fixed.
… AgentInfo object. The Agent UTs has also been fixed.
4b26e04
to
513c83f
Compare
…intaining the naming convention and correctly checking the contents of json objects.
… the naming convention
513c83f
to
150e1e1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should check all ocurrences of "persistance" and change it to "persistence" to keep some consistency
target_link_libraries(agent_info_test PRIVATE AgentInfo Persistence GTest::gtest) | ||
target_include_directories(agent_info_test PRIVATE | ||
${CMAKE_CURRENT_SOURCE_DIR}/../src | ||
${CMAKE_CURRENT_SOURCE_DIR}/../../persistence/tests/mocks) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note for the future: we need to do this also with tests that use the http client mock.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Hi @jr0me, I have created another issue to work on this.
|
Description
This PR adds the necessary changes to avoid the use and/or creation of the
agent_info.db
database in the UTs.Proposed Changes
The Persistence interface is used to declare a mock class(MockPersistence) that implements this interface in order to mock the methods used by AgentInfoPersistance
The changes include:
Results and Evidence
Running of the Agent:
Agent Registration:
Agent Run:
AgentInfoPersistance Test:
AgentInfo Test:
AgentRegistration Test:
Agent Test:
As you can see, the agent_info.db database has not been created. The use of the other dbs in the UTs will be corrected in another issue.
Artifacts Affected
Executable files, all platforms.
Configuration Changes
None
Documentation Updates
None
Tests Introduced
Added:
Fixed:
Review Checklist